Skip to content

Network batching - #631

Merged
MykolaSuperman merged 16 commits into
aosedge:developfrom
MykolaSuperman:network-batching
Aug 4, 2026
Merged

Network batching#631
MykolaSuperman merged 16 commits into
aosedge:developfrom
MykolaSuperman:network-batching

Conversation

@MykolaSuperman

Copy link
Copy Markdown

No description provided.

@MykolaSuperman
MykolaSuperman force-pushed the network-batching branch 4 times, most recently from d5515bd to e91ea36 Compare July 24, 2026 10:02
EXPECT_CALL(mBandwidth, Clear(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone));
EXPECT_CALL(mFirewall, RemoveInstance(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone));
EXPECT_CALL(mBridgeNetwork, Detach(_, _)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone));
// The host veth is no longer detached synchronously on stop; the instance

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment makes no sense without the removed line.

Comment thread src/core/sm/launcher/launcher.cpp Outdated
Launcher::InstanceData* Launcher::FindInstanceDataByID(const String& instanceID)
{
auto it = mInstances.FindIf([&instanceID](const auto& instance) { return instance.mInstanceID == instanceID; });

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove empty line

@mykola-kobets-epam mykola-kobets-epam left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>

@MykolaSuperman
MykolaSuperman force-pushed the network-batching branch 2 times, most recently from 18b983d to 4ce7f00 Compare August 3, 2026 15:37

@mlohvynenko mlohvynenko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>

@al1img al1img left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Mykola Solianko added 16 commits August 4, 2026 11:26
…nc detach

Stop no longer deletes the host veth synchronously. DeleteNetworkNamespace
drops the instance netns (lazy MNT_DETACH umount); the kernel then reaps the
peer veth - and the host end with it, since they die as a pair -
asynchronously via cleanup_net, off the critical stop path.

The synchronous per-instance delete blocked on a kernel RCU grace period for
every instance, serialized under rtnl_lock (O(N) on mass teardown): measured
~2.5s to delete 200 host veths one-by-one vs ~0.25s for the lazy namespace
teardown (which also lets cleanup_net batch the unregister). No leak: the
interfaces are gone once the namespace is reaped.

Detach is kept for the AddInstanceToNetwork rollback path, where the namespace
may not yet own the peer.

Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
DeleteInstanceNetworkConfig only calls Bandwidth::Clear when the instance
config actually had a non-zero ingress/egress limit. For unlimited instances
(the common case) nothing was installed, so the previous unconditional Clear
just wasted three tc round-trips (serialized on rtnl_lock) per instance on a
mass teardown. The decision uses the config already held in the network
manager, so no extra state is kept in the bandwidth backend.

Tests to be updated separately.

Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
…ilure

Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
…stances

Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
… fallback

Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Discarding a staged batch without applying it is not expressible with
FlushBatch/Revert: FlushBatch commits and Revert undoes an already
applied batch. Add AbortBatch to both backend interfaces and mocks.

Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
FlushBatch commits the firewall, traffic-monitor and storage backends in
sequence and gives each failure its own recovery so a partial batch never
leaves one backend applied while another is not: a firewall-flush failure
aborts the still-staged traffic batch, a traffic-flush failure reverts the
already-flushed firewall, and a storage-commit failure reverts both. The
storage transaction is rolled back on every failure path, and the batch
entries are re-applied per instance so one bad instance is isolated
instead of failing the whole flush.

Cover the firewall-, traffic- and commit-failure paths with tests.

Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
ReconcileInstances registered the network DNS server (AdoptDNSServer) only
on the leftover-cleanup path, not for instances adopted as running via
InitInstance. DeleteInstanceNetworkConfig then could not find the DNS
server ("DNS server not found for cleanup") and left stale addnhosts
entries when such an instance was later removed.

Adopt the DNS server before continuing the running-instance branch too.
AdoptDNSServer is idempotent, so multiple instances on one network are
safe; a failure is logged and does not abort adoption.

Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Adopt a running leftover instance on restart, assert the network DNS
server is registered (CreateServer), and that a subsequent
StopInstanceNetwork drops the instance host entry (RemoveHost). Without
the reconcile fix neither call happens, so the test fails.

Update Start_KeepsLeftoverInstanceWithLiveInterface to expect the DNS
server adoption too.

Signed-off-by: Mykola Solianko <mykola_solianko@epam.com>
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 4, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
44.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@MykolaSuperman
MykolaSuperman merged commit 9208a0a into aosedge:develop Aug 4, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants